security(backend): enforce token scopes consistently across transfer and admin routes - #143
Open
woahwhattheheck wants to merge 3 commits into
Open
woahwhattheheck wants to merge 3 commits into
woahwhattheheck wants to merge 3 commits into
Conversation
Add a canonical scope catalog, re-check scopes at service boundaries, gate admin diagnostics on explicit admin:read (while mapping the legacy admin key), and introduce bulk transfer mutations with non-enumerating per-id errors so malformed and missing ids cannot be told apart. Closes RemitFlow#125
Reject inherited action names and non-string values before handler lookup. Preserve scope checks, unsupported-string errors, and valid bulk behavior. Five HTTP regressions reproduce false success, a coerced cancellation, and a malformed-action 500 on the prior source. Full npm test: 281 pass.
Fail startup for malformed or invalid supplied token maps instead of enabling public demo credentials. Validate configured scopes using the existing catalog and keep token lookup limited to explicit entries. Preserve absent-variable demo mode, valid maps, and the legacy admin credential behavior. Validation: 16 new failure-mode cases failed on original source; npm test now passes all 298 tests without skips. Actual local production-mode Express runs confirmed malformed/empty maps previously granted demo admin access and array-shaped maps granted token 0. All invalid configurations now stop startup; valid-map and absent-variable controls still pass. No dependency changes or external provider calls.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #125.
Enforces the documented token-scope contract consistently across transfer, user, audit, and admin surfaces so a token can perform only the actions it is granted, admin paths require an explicit scope, and unauthorized responses stay non-enumerating.
What
src/config/scopes.js(transfers:*,users:*,audit:read,admin:read) with a frozen route matrix.src/utils/authz.js) re-check scopes at service boundaries; route middleware remains the first gate.admin:read. LegacyADMIN_API_KEY/X-Admin-Tokenis mapped onto[admin:read]so the path stays scope-gated without breaking existing operators.POST /api/transfers/bulkfor claim/cancel/archive/unarchive (max 50 ids) with per-id results.404 Transfer not foundenvelope (direct and bulk) so callers cannot enumerate identifiers by status or message shape.docs/SCOPE_MATRIX.md, README scope table, CHANGELOG.Acceptance criteria mapping
assertScopesadmin:readvia scoped API token or legacy key mapped to that scopetest/tokenScopes.test.jsnpm test— 275 passDesign tradeoffs / compatibility
transfers:readtoken (preserves cursor actor-binding tests and the existing shared-data demo model). Cross-surface isolation (transfers token ↛ admin/audit/users writes) is what closes the exposure called out in security(backend): enforce token scopes consistently across transfer and admin routes #125.admin:readrather than a parallel auth system.authand stay trusted; HTTP controllers always passauthFromRequest(req).How tested
Coverage added: scope matrix, cross-surface isolation, bulk auth + non-enumerating per-id errors, malformed vs missing id parity, audit authorization (HTTP + service), service-boundary under-scope rejection, admin:read via API token and legacy key.